Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/stagingBuildTimes #1008

Merged
merged 5 commits into from
Nov 2, 2023
Merged

Feat/stagingBuildTimes #1008

merged 5 commits into from
Nov 2, 2023

Conversation

kishore03109
Copy link
Contributor

@kishore03109 kishore03109 commented Nov 1, 2023

Problem

BE for staging build status. Corresponding FE pr

Closes [insert issue #]

Solution

Get the latest commit as SOT.

Tests

done in fe

@kishore03109 kishore03109 changed the base branch from develop to fix/quickie/dbUpdate November 1, 2023 14:48
@kishore03109 kishore03109 marked this pull request as ready for review November 1, 2023 18:11
@kishore03109 kishore03109 requested a review from a team November 1, 2023 18:11
Copy link
Contributor

@dcshzj dcshzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking comments

siteId,
},
}),
() => new NotFoundError("Site has not been deployed!")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be a different error since it means the DB couldn't be queried.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait no in this case the deployments row doesnt exist, so it is the case that the site has not been deployed!

@@ -0,0 +1,10 @@
export const statusStates = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be fully capitalised with snake case

isReduceBuildTimesWhitelistedRepo(growthbook)
)
)
.andThen((status) => okAsync({ status }))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should use .map instead

@@ -37,6 +37,7 @@ import {
SiteLaunchStatusObject,
} from "@root/types/siteInfo"
import { SiteLaunchMessage } from "@root/types/siteLaunch"
import { isReduceBuildTimesWhitelistedRepo } from "@root/utils/growthbook-utils"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unused?

@kishore03109 kishore03109 changed the base branch from fix/quickie/dbUpdate to develop November 2, 2023 07:13
@kishore03109 kishore03109 force-pushed the feat/stagingBuildTimes branch from ba8ac43 to 666f8a6 Compare November 2, 2023 07:15
@kishore03109 kishore03109 merged commit 64d61e0 into develop Nov 2, 2023
8 checks passed
@mergify mergify bot deleted the feat/stagingBuildTimes branch November 2, 2023 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants